Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix duplicated Content-Type values #247

Merged

Conversation

massdonati
Copy link

fix issue #246

@massdonati massdonati force-pushed the fix/duplicated_contentType_value branch from 3fdb631 to a9bcda7 Compare June 11, 2018 17:21
@pcantrell pcantrell merged commit 27dd4f9 into bustoutsolutions:master Jun 22, 2018
@@ -25,9 +31,8 @@ public extension Resource
{
underlyingRequest in

underlyingRequest.addValue(contentType, forHTTPHeaderField: "Content-Type")
underlyingRequest.setValue(contentType, forHTTPHeaderField: "Content-Type")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It took me a while digging into Siesta 1.4 to understand what happened to my failing request.
I arrived here @pcantrell and I think this will break quite some code around.

At some point in my app I have a Resource R1 I need to load using a Resource R2.
R1.load(using: R2.withParam("someParam", value).request(.post, text: "SomeText") )
Doing this the request contentType param has its default "text/plain" value

The change-set here is going to overwrite completely the contenType stored by my Service Resource Configuration.
I don't think this is a wanted behaviour. Isn't it?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this can be a viable solution. I Can't produce a fully tested PR due to my limited time. Sorry about it.

if underlyingRequest.value(forHTTPHeaderField: "Content-Type")?.contains(contentType) == false {
    underlyingRequest.addValue(contentType, forHTTPHeaderField: "Content-Type")
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants